Skip to content

Support stdin with LSF bsub#347

Merged
mrocklin merged 4 commits into
dask:masterfrom
mrocklin:lsf-stdin
Oct 7, 2019
Merged

Support stdin with LSF bsub#347
mrocklin merged 4 commits into
dask:masterfrom
mrocklin:lsf-stdin

Conversation

@mrocklin

@mrocklin mrocklin commented Oct 4, 2019

Copy link
Copy Markdown
Member

No description provided.

@lesteve

lesteve commented Oct 5, 2019

Copy link
Copy Markdown
Member

Looks good to me, @SimonBoothroyd, can you confirm that this PR works for you?

@mrocklin I assume you have tested this on Summit for LSF 10?

@SimonBoothroyd

Copy link
Copy Markdown

@lesteve I tested the config route and it works great for me! But the lsf_version route raises a TypeError: cannot use a string pattern on a bytes-like object. I think also the regex pattern will match 10.1.0.7 as 0.1.0.7 but something more like

@toolz.memoize
def lsf_version():
    out, _ = subprocess.Popen("lsid", stdout=subprocess.PIPE).communicate()
    version = re.search(r"(\d+\.)+\d+", out.decode()).group()
    return LooseVersion(version)

should work. From a quick test of this it also seems like use_stdin should just compare < "10" rather than <= "9" to handle cases like 9.1.XXX, but otherwise this is great - thanks for making this change @mrocklin !

@mrocklin

mrocklin commented Oct 5, 2019

Copy link
Copy Markdown
Member Author

@mrocklin I assume you have tested this on Summit for LSF 10?

I didn't before (my apologies) but I have now, and yes, things work with the recent changes suggested by @SimonBoothroyd

@mrocklin mrocklin merged commit 8d836a3 into dask:master Oct 7, 2019
@mrocklin mrocklin deleted the lsf-stdin branch October 7, 2019 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants